Fix cross-platform injection and launcher compatibility - #110
Conversation
|
Does this PR obsolete #109? Looks like the mac build is failing, can you look into it? |
#110 fully supersedes #109, so #109 can be closed once #110 is merged. The macOS failure is caused by |
<mach-o/dyld-interposing.h> is part of the dyld source tree but is not shipped in the Xcode SDK, so the include failed on CI runners. Vendor the stable macro definition instead; the generated __DATA,__interpose tuples are identical.
-lUnityPlayer only resolves lib-prefixed library names, while the caller check requires the image basename to start with UnityPlayer. Link the dylib by explicit path so its @rpath install_name is used instead.
load_mono_funcs/ load_il2cpp_funcs probe many optional symbols, leaving the last failed probe in dlerror. Callers using the textbook dlerror()-after-dlsym() pattern then see a phantom error even though their own lookup succeeded. Drain the error state when the caller's lookup succeeded; surfaced by the macOS interposition smoke test.
The macOS build failure is fixed. CI is now green on all three platforms (verified on the fork's identical workflow: https://github.com/mohui666/UnityDoorstop/actions/runs/32021379965). Root cause and fixes, three commits:
With these in place, the new Note: the workflow run on this PR shows |
|
@copilot review |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 31 out of 31 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
tests/windows/config-util-regressions.c:3
- Duplicate include of ../../src/bootstrap.h. While the header is include-guarded, this is unnecessary and can hide accidental copy/paste errors in test sources.
#include "../../src/bootstrap.h"
#include "../../src/bootstrap.h"
tests/windows/run-regressions.ps1:6
- run-regressions.ps1 unconditionally resolves gcc.exe and throws if it isn't present. Since the script passes gcc-style flags, consider allowing override via an env var (e.g. CC) pointing to a gcc/clang-compatible compiler, and emit a clearer error when neither gcc.exe nor CC are available.
$compiler = (Get-Command gcc.exe -ErrorAction Stop).Source
| - name: Run Windows regression tests | ||
| run: ./tests/windows/run-regressions.ps1 | ||
| - name: Build Release |
|
I don't believe whichever LLM created this PR has enough context to make a meaningful contribution. I'm stronger with the shell script and build pipeline so I will focus there for now. At 11c790d#diff-672ab20553b2eb1be64f57ff9b63eb32799c1ecfd10707b184f9da8b2ca331caL67, the comment pointing to #88 is removed and the PR description says that #88 is fixed, however the code block immediately after that comment is left intact. That code block only exists to work around #88 and should be removed and tested if it is truely fixed. At 11c790d#diff-672ab20553b2eb1be64f57ff9b63eb32799c1ecfd10707b184f9da8b2ca331caR382 the DYLD_INSERT_LIBRARIES envvar is unset which is the literal fix to #107, it does not address the concerns over DYLD_LIBRARY_PATH also needing to be handled. It also does not address the problems from the loaded binaries not fully supporting all platforms as present when used with BepInEx. The script likely needs to have a way to be configured for exactly which architectures the game and modding framework support so that it can be configured per game. I have been brainstorming on this but have not made any changes yet as I dont have a mac to test the changes on. There are also many unexplained changes to run.sh that I would like to know why they are being suggested. I believe they are related to #84 which is not a valid issue needing to be fixed. The added unit tests are being run from standalone scripts calling compilers directly. The project uses xmake and any tests should be incorporated into the xmake configuration for consistent execution. |
Summary
This consolidates the reproducible cross-platform compatibility fixes from the current issue backlog.
LD_PRELOADno longer breaks shell command substitution or file redirectionrun.shDYLD_INSERT_LIBRARIESfrom the Apple SiliconarchhelperGetProcAddressIAT hooks and harden PE/CLI/path parsingFixes #88.
Addresses #24, #34, #47, #67, #73, #84, #94, #98, #102, #107, and #108.
Safety and compatibility
The macOS chained-fixups path no longer maps import ordinals to guessed GOT indices.
dlsym, boot.config access, and stdout protection use dyld interposition instead; stdio interception checks that the direct caller is UnityPlayer, so inherited injection does not affect launchers or shells.Missing target assemblies now skip only managed entrypoint execution. They no longer disable independent debugger, boot.config, and search-path functionality.
Verification
LD_PRELOAD, shell redirection, UnityPlayer dup2, path, and restart-marker regressionsgit diff --checkCI additionally runs the real macOS dyld interposition smoke test and the normal Linux x86 build in its multilib runner.